Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for JOR files #141

Draft
wants to merge 3 commits into
base: gcos4gnucobol-3.x
Choose a base branch
from

Conversation

lefessan
Copy link
Member

JOR files are journals of COBOL executions. This commit implements
minimalist support for JOR files with the following features:

  • extensible: the format can be easily extended in the future
  • compact: the format is binary and does not take too much space on disk

Currently, the JOR file contains the arguments, the execution start
and stop times and per-file statistics of opens, reads, writes, starts
and closes operations.

Initial work by Emilien Lemaire <[email protected]>

Additional features:
* Do not check for enter/exit section: instead, sum time from paragraphs
* Support for modules, CALL and ENTRY points
* Support for recursive calls
* Allocate virtual stack on demand instead of statically
* Correct handling of EXIT PARAGRAPH code with 'goto'
* Prevent CANCEL from dlclose a module during profiling
* Customize CSV result file with COB_PROF_FORMAT
* Customize CSV filename using $b/$f/$d/$t
* Add some tests for RECURSIVE on PROGRAM-ID
@lefessan lefessan marked this pull request as draft March 20, 2024 10:19
Copy link
Collaborator

@GitMensch GitMensch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

obviously this has one commit too much in currently; for the first draft see some notes in the review; the biggest question is (likely answered in NEWS and gnucobol.texi): why should a user want that, what is its practical purpose?

Should there be binaries under /bin that take an existing jor file and convert it to xml/json/text?

@@ -3050,6 +3056,7 @@ cob_stop_run (const int status)
longjmp (return_jmp_buf, 1);
}
#endif
cob_jor_exit (status, "STOP RUN reached");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you'd like to log the status here as well

libcob/common.h Outdated
Comment on lines 2880 to 2893
enum cob_jor_file_operation {
/* order must match field_file_op_name[] in jor.c */
COB_JOR_WRITE_TRY = 0,
COB_JOR_WRITE_OK,
COB_JOR_READ_TRY,
COB_JOR_READ_OK,
COB_JOR_START_TRY,
COB_JOR_START_OK,
COB_JOR_OPEN_TRY,
COB_JOR_OPEN_OK,
COB_JOR_CLOSE_TRY,
COB_JOR_CLOSE_OK,
COB_JOR_AFTER_LAST_OPERATION
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put that above, add a trailing "_MAX" entry and use that max entry as index instead of COB_JOR_FILE_OPERATIONS

Comment on lines +2895 to +2903
COB_EXPIMP void cob_jor_exit (int, const char*, ...);
COB_EXPIMP void cob_jor_file_operation (cob_file*, enum cob_jor_file_operation);

/* For extensions to update the way JOR are allocated and saved/sent. */
struct cob_jor_funcs {
void (*save) (const char* filename, char* buffer, int len);
char* (*allocate)(int *size);
};
COB_EXPIMP void cob_jor_set_funcs (struct cob_jor_funcs *);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's not external, so move tgat part to coblocal

libcob/jor.c Outdated
Comment on lines 2 to 3
Copyright (C) 2023 Free Software Foundation, Inc.
Written by Emilien Lemaire and Fabrice Le Fessant.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you did start that last year already, together with Emilien?

Comment on lines +21 to +51
#include <limits.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <stdarg.h>

#include "config.h"

/* include internal and external libcob definitions, forcing exports */
#define COB_LIB_EXPIMP
#include "coblocal.h"

#include "tarstamp.h"
#include "common.h"

#include <sys/types.h>
#ifdef HAVE_UNISTD_H
#include <unistd.h>
#endif

#ifdef _WIN32
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#undef MOUSE_MOVED
#include <process.h>
#include <io.h>
#include <fcntl.h> /* for _O_BINARY only */
#endif

#include <sys/time.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as clangd and some compilers will tell you - these are not all used in the source, please cleanup

libcob/jor.c Outdated
Comment on lines 316 to 328
if (COB_JOR_FILE_OPERATIONS != COB_JOR_AFTER_LAST_OPERATION){
fprintf (stderr, "COB_JOR_FILE_OPERATIONS = %d\n",
COB_JOR_FILE_OPERATIONS);
fprintf (stderr, "COB_JOR_AFTER_LAST_OPERATION = %d\n",
COB_JOR_AFTER_LAST_OPERATION);
exit (2);
}
if ( field_file_op_name[COB_JOR_FILE_OPERATIONS-1] == NULL ){
fprintf (stderr,
"field_file_op_name[%d] not initialized\n",
COB_JOR_FILE_OPERATIONS-1);
exit (2);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the common abort and message functions here or drop that part

Comment on lines +330 to +332
if (!cobsetptr->cob_jor_enable &&
/* testsuite clears COB_JOR_ENABLE... */
!getenv ("COB_JOR_ENABLED")) return ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that will be already be handled in the settings pointer, so this part can be removed

libcob/fileio.c Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cob_jor_file_operation is called often, so I think we may check !cobsetptr->cob_jor_enable in this file instead (saving the function call and its resulting stack/register changes on in and out), possibly wrap that in a macro so you keep the single lines here. But I've done no perf tracing on this...

libcob/jor.c Outdated
{
int i;

if (!!cobsetptr) return ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would be an error, I therefore would suggest to drop that

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the outline at the start is well done; please add doxygen comments to each of the functions as well

@GitMensch
Copy link
Collaborator

What do you think of moving the stats to be only/mostly set on save_status as per Ron's implementation OCamlPro/gnucobol4@4a61f7c OCamlPro/gnucobol4@24a3751?

What about merging the fileio stats (only active for programs that are compiled with tracing) to GC 3.3 (that would also include OCamlPro/gnucobol4@980d0b4)?

@lefessan
Copy link
Member Author

The idea behind this PR is to be able to get a minimal trace of the execution, in production mode. This is not possible with standard tracing options that are too expensive. You can imagine a batch failing during the night, people might want to understand what happened exactly during the run, and maybe to preceding steps, without running them again in trace mode. Also, you get information like the time spent, the IO stats, and probably more in the future.

There are maybe other solutions, but I think it's worth investigating this mode.

And yes, there needs to be some program that will translate these binary files into json or something like that...

By the way, I am not really targeting 3.3, ideally, it's something that could land in 4.0 once 3.3 has been released.

@GitMensch
Copy link
Collaborator

By the way, I am not really targeting 3.3, ideally, it's something that could land in 4.0 once 3.3 has been released.

Ah, then please edit the PR to have that based on master then.
Still, it would be useful to check the review comments, including the question to do it at the same place: save_status and in 4.x consider to provide the option to disable/enable the file recording by file (using the IO_ options, see referenced commits).

This is not possible with standard tracing options that are too expensive.

We still can consider to use these as basis -> check for a new trace value "binary" and in this case do the tracing you currently do.

In any case I suggest to write a plan (which includes your binary format), then send that to the developer mailing list for additional input - especially when you target trunk.

JOR files are journals of COBOL executions. This commit implements
minimalist support for JOR files with the following features:
* extensible: the format can be easily extended in the future
* compact: the format is binary and does not take too much space on disk

Currently, the JOR file contains the arguments, the execution start
and stop times and per-file statistics of opens, reads, writes, starts
and closes operations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants